Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix Style linting issues #2495

Merged
merged 14 commits into from
Dec 15, 2021
Merged

Fix Style linting issues #2495

merged 14 commits into from
Dec 15, 2021

Conversation

averdin2
Copy link
Member

@averdin2 averdin2 commented Nov 17, 2021 •
edited
Loading

Fixes #1958

What changes did you make and why did you make them?

  • I fixed various style issues that were found from the implementation of a GitHub Action for lining style issues.
  • Some code changes (CSS) were needed to fix the issues.
  • I had to take some liberties in deciding how the website looked after making changes as some of the errors caused the site to behave in an unintended way that had previously not been caught.
  • I still need to make changes to one file before merging.

Notes

  • See the issue for some more explanation on some of the changes made.
  • This issue was needed after issue Implement SCSS lint #1441 was completed.

@averdin2 averdin2 requested a review from macho-catt November 17, 2021 02:52
@github-actions
Copy link

Want to review this pull request? Take a look at this documentation for a step by step guide!

From your project repository, check out a new branch and test the changes.

git checkout -b averdin2-1958-fix-css-linting-issues gh-pages
git pull https://github.com/averdin2/website.git 1958-fix-css-linting-issues

@github-actions github-actions bot added Collaborative Work Feature: Board/GitHub Maintenance Project board maintenance that we have to do repeatedly Feature: Refactor CSS Page is working fine - CSS needs changes to become consistent with other pages role: back end/devOps Tasks for back-end developers Complexity: Large Status: Updated No blockers and update is ready for review labels Nov 17, 2021
macho-catt
macho-catt suggested changes Nov 19, 2021
View reviewed changes
Copy link
Member

@macho-catt macho-catt left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @averdin2, thanks on working on this. Just a couple of change requests on two pages:

events.scss

Just set the margin to zero, because on mobile view the 52 margin top was originally being overwritten to be all zeroes, but because you're using margin 52 for the top again, there is unnecessary space on the container.

toolkit.scss

Could you revert all the changes you suggested here? It's causing some weirdness on the mobile view (the header is misaligned with the button, and the guides are separated into two columns instead of being flexed), and I think it should be fine as is:

Click for screenshot

Screenshot 2021-11-18 21:01:26

Please lmk if you have questions, thanks!

macho-catt
macho-catt approved these changes Dec 15, 2021
View reviewed changes
Copy link
Member

@macho-catt macho-catt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thank you for all your work on this!

@macho-catt macho-catt merged commit 998b164 into hackforla:gh-pages Dec 15, 2021
@roslynwythe roslynwythe mentioned this pull request Nov 22, 2023
Closed
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@macho-catt macho-catt macho-catt approved these changes

Assignees
No one assigned
Labels
Complexity: Large Feature: Board/GitHub Maintenance Project board maintenance that we have to do repeatedly Feature: Refactor CSS Page is working fine - CSS needs changes to become consistent with other pages role: back end/devOps Tasks for back-end developers Status: Updated No blockers and update is ready for review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Make Style Linting Fixes
3 participants
@averdin2 @macho-catt @ExperimentsInHonesty